Skip to content

Wire TypeScript interpreter module into state.ts, remove Go endpoint#3704

Open
zackverham wants to merge 5 commits intomainfrom
interpreters-wiring
Open

Wire TypeScript interpreter module into state.ts, remove Go endpoint#3704
zackverham wants to merge 5 commits intomainfrom
interpreters-wiring

Conversation

@zackverham
Copy link
Collaborator

@zackverham zackverham commented Mar 12, 2026

Summary

  • Replaces api.interpreters.get() HTTP calls with in-process getInterpreterDefaults() from the TypeScript interpreter module (Add TypeScript interpreter detection module #3703)
  • Adds requiresPython / requiresR fields to PythonConfig / RConfig types and fill-in logic in UpdateConfigWithDefaults
  • Removes the Go /api/interpreters endpoint (get_interpreters.go) and its route registration
  • Removes the Interpreters API resource class from the TypeScript client

Bug fix: requiresPython / requiresR were never applied to configurations

The Go FillDefaults methods (config/types.go) already populated requiresPython and requiresR on the interpreter response DTO. However, the TypeScript UpdateConfigWithDefaults only copied version, packageFile, and packageManager from the defaults — it silently dropped the requiresPython / requiresR fields. This PR adds the missing fill-in logic so these fields are actually applied to configurations for the first time.


This is PR 2 of 3 splitting up #3672. This is the behavioral switchover — can be reviewed independently of PR 3.

PRs: PR 1 (#3703, merged) | PR 2 (this) | PR 3 (#3705)

Test plan

  • npx tsc --noEmit passes
  • npx vitest run src/api/types/configurations.test.ts — 12 tests including 6 new tests for requiresPython / requiresR fill-in logic
  • npx vitest run src/state.test.ts — 16 tests pass
  • go test -short ./internal/services/api/... passes
  • npx prettier --check passes
  • CI passes

🤖 Generated with Claude Code

@zackverham zackverham requested a review from a team as a code owner March 12, 2026 13:54
@zackverham zackverham force-pushed the interpreters-module branch from 9103644 to 60901e6 Compare March 12, 2026 14:26
@zackverham zackverham force-pushed the interpreters-wiring branch from d859387 to 9728cb9 Compare March 12, 2026 15:12
Base automatically changed from interpreters-module to main March 12, 2026 19:18
Replace the HTTP call to the Go /api/interpreters endpoint with an
in-process call to the TypeScript getInterpreterDefaults() function
added in the previous PR.

Changes:
- configurations.ts: add requiresPython/requiresR fields to
  PythonConfig/RConfig types and fill-in logic in UpdateConfigWithDefaults
- state.ts: use getInterpreterDefaults() instead of api.interpreters.get()
- client.ts: remove Interpreters resource import and property
- Delete Interpreters.ts API resource class
- Delete get_interpreters.go and its tests
- Remove /api/interpreters route from api_service.go

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@zackverham zackverham force-pushed the interpreters-wiring branch from 9728cb9 to 79ac36e Compare March 12, 2026 19:24
…ithDefaults

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@zackverham zackverham requested a review from christierney March 12, 2026 20:14
if (!config.configuration.r.packageManager) {
config.configuration.r.packageManager = defaults.r.packageManager;
}
if (!config.configuration.r.requiresR) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this (and the python equivalent below) seem obviously right, but I'm surprised this wasn't already here? This function predates the migration project. Was there an existing bug here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, yeah - I believe this was a bug. I should add that to the PR description, sorry.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're not just moving, we're improving!

@zackverham
Copy link
Collaborator Author

Smoke Test Plan

What changed

This PR replaces the HTTP call to the Go /api/interpreters endpoint with an in-process TypeScript call (getInterpreterDefaults()), removes the Go endpoint entirely, and adds requiresPython/requiresR fill-in logic to UpdateConfigWithDefaults.

Two code paths in state.ts are affected:

  1. refreshConfigurations() — called on startup, when .posit/publish/*.toml files change, and on manual refresh
  2. getSelectedConfiguration() — called when rendering content, refreshing packages, managing secrets, editing configs, deploying, etc.

Prerequisites

  • A workspace with at least one .posit/publish/*.toml configuration that has a python and/or r section
  • Python and/or R installed and on PATH
  • Optionally: a project with .python-version, pyproject.toml, DESCRIPTION, or renv.lock for requiresPython/requiresR coverage

Test cases

1. Extension startup — Python project

  1. Open a project that has a .posit/publish/*.toml config with a python section
  2. The sidebar should load without errors
  3. The Python packages panel should display — confirming python.version, python.packageFile, and python.packageManager were populated from the TypeScript detector

Pass: Sidebar renders, Python packages section shows the correct package file and packages. No errors in the Output panel.

2. Extension startup — R project

  1. Open a project with a .posit/publish/*.toml config that has an r section
  2. The R packages panel should display correctly

Pass: R packages section shows packages. No errors.

3. Configuration file change triggers refresh

  1. With the extension running, edit a .posit/publish/*.toml file (e.g. change the title)
  2. The file watcher should trigger refreshConfigurations()
  3. The sidebar should update without errors

Pass: Changes reflected in sidebar. No errors in Output panel.

4. Selecting a deployment loads its configuration

  1. If multiple deployments exist, switch between them in the sidebar
  2. Each switch calls getSelectedConfiguration(), which invokes getInterpreterDefaults()

Pass: Configuration loads for each deployment. Python/R package sections update appropriately.

5. Python not on PATH

  1. Temporarily rename/remove Python from PATH (or test on a machine without Python)
  2. Open a project with a Python configuration

Pass: Extension loads without crashing. The Python section should still appear but with empty/default version. No unhandled exceptions.

6. R not on PATH

  1. Same as above but for R

Pass: Extension loads without crashing. R section gracefully handles missing interpreter.

7. Mixed project (Python + R)

  1. Open a project whose .toml config has both python and r sections
  2. Both package panels should populate correctly

Pass: Both Python and R packages display. Both versions are detected.

8. Deploy still works

  1. Select a deployment and click Deploy
  2. The deployment should proceed — this exercises the full config-loading pipeline including interpreter defaults

Pass: Deployment completes successfully. The deployed configuration has correct interpreter versions.

9. Go endpoint is gone (negative test)

  1. With the extension running, attempt to call GET /api/interpreters?dir=. directly via curl against the Go backend port

Pass: Returns 404 or "not found" — confirming the route was removed and nothing else depends on it.

10. requiresPython / requiresR populated

  1. Create a project with a .python-version file (e.g. 3.11) and a DESCRIPTION file with Depends: R (>= 4.1.0)
  2. Open the project and check the configuration that gets written during deployment

Pass: The requiresPython and requiresR fields are populated in the configuration defaults.

Automated coverage

Area Coverage
UpdateConfigWithDefaults fill-in logic configurations.test.ts (12 tests including 6 new)
state.ts mock paths state.test.ts (16 tests)
Interpreter detection module interpreters/*.test.ts (unit), integration.test.ts (PR #3705)

@zackverham
Copy link
Collaborator Author

Root cause analysis: relative projectDir passed to TypeScript interpreter module

The bug

After replacing the Go /interpreters endpoint with the new TypeScript interpreter detection module (src/interpreters/), the sidebar would erroneously show the Scan button for projects that have a requirements.txt. The interpreter defaults (e.g. packageFile: "requirements.txt") were coming back empty even though the file existed on disk.

Root cause

The two call sites in state.ts that invoke getInterpreterDefaults() were passing relative projectDir values:

  1. getSelectedConfiguration() passed contentRecord.projectDir — a relative path like "report-GUD1"
  2. refreshConfigurations() passed the literal string "."

The TypeScript module uses these paths for filesystem operations (fileExistsAt, execFile cwd), which resolve relative to process.cwd() of the extension host process — not the user's workspace root. So requirements.txt detection was looking in the wrong directory and always failing.

The old Go endpoint received the same relative paths but resolved them server-side against its known base directory, so it worked correctly. When we removed that server-side resolution by moving to TypeScript, we lost that implicit path fixup.

The fix

Resolve projectDir to an absolute path in state.ts before passing it to the TypeScript module:

  • getSelectedConfiguration(): path.join(root, contentRecord.projectDir) — joins the workspace root with the content record's relative project dir
  • refreshConfigurations(): pass root directly instead of "."

root comes from workspaces.path() which returns the workspace folder's fsPath (already absolute).

Test coverage added

  • state.test.ts: Asserts getInterpreterDefaults receives /workspace/<projectDir> (absolute) in the getSelectedConfiguration test, and "/workspace" (not ".") in a new refreshConfigurations test
  • integration.test.ts: Added an explicit test confirming getInterpreterDefaults correctly detects requirements.txt when given an absolute projectDir path

…efaults

The TypeScript interpreter module uses projectDir for filesystem operations
that resolve relative to process.cwd(), not the workspace root. The old Go
endpoint handled this server-side. Fix by joining workspace root with
projectDir in state.ts before calling getInterpreterDefaults.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use path.join() in the test assertion instead of a template literal
with forward slashes, so the expected value matches on Windows where
path.join produces backslashes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants